Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Actions] Adjust axios payload for APIs that do not want data sent #165756

Merged
merged 6 commits into from
Sep 6, 2023

Conversation

stephmilovic
Copy link
Contributor

@stephmilovic stephmilovic commented Sep 5, 2023

Problem: Cases Webhook Connector throwing vague error

A customer reported issues while using the Webhook - Case Management connector (cases_webook). This is the error they saw:

Screenshot 2023-09-05 at 12 36 49 PM

After gaining access to an instance of TheHive5, I was able to narrow down the issue to the GET incident call. When I removed the data object from being sent as {} when undefined, the error went away and the call succeeded. I used curl against the Hive5 API directly. As suspected, when a data object is included the API throws: {"type":"NotFoundError","message":"/api/v1/case/~122917056"}. It seems likely axios swallowed that error.

Solution: Removing data when undefined

As mentioned above, I am removing the axios data argument instead of being sent as {} when undefined. A GET request doesn't usually transmit data in any other way besides passing an encoded string in the URL. I think it makes sense to not send data when it is undefined vs sending an empty object as it can break some 3rd party APIs as we are seeing with Hive5

To test

  1. Use these TheHive5 credentials. to create a cases webhook connector.

  2. Run the test, post a sample case.

  3. Confirm you see a successful post:
    Screenshot 2023-09-05 at 12 58 29 PM

  4. Extra confirm by logging into the hive with the above credentials and see the case you created there

@stephmilovic stephmilovic requested a review from a team as a code owner September 5, 2023 18:59
@stephmilovic stephmilovic added release_note:fix Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.10.0 v8.11.0 labels Sep 5, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! thx

It's just sooo wrong to send any data on a GET, but ES actually requires it for some calls. I think a number of servers and gateways actually choke on this! I imagine we've not seen problems as nearly every service being used by the connectors uses POST ...

@stephmilovic
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine kibanamachine requested a review from a team as a code owner September 5, 2023 20:46
@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Sep 5, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@stephmilovic stephmilovic removed the request for review from a team September 5, 2023 20:55
@stephmilovic
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@shahzad31 shahzad31 removed the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Sep 6, 2023
@stephmilovic stephmilovic merged commit d6a2bd7 into elastic:main Sep 6, 2023
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 6, 2023
kibanamachine added a commit that referenced this pull request Sep 6, 2023
…` sent (#165756) (#165851)

# Backport

This will backport the following commits from `main` to `8.10`:
- [[Actions] Adjust axios payload for APIs that do not want `data` sent
(#165756)](#165756)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Steph
Milovic","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-09-06T13:38:39Z","message":"[Actions]
Adjust axios payload for APIs that do not want `data` sent
(#165756)","sha":"d6a2bd77788c318a9f74247a52a1b6da05779b4b","branchLabelMapping":{"^v8.11.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:
SecuritySolution","v8.10.0","v8.11.0"],"number":165756,"url":"https://github.com/elastic/kibana/pull/165756","mergeCommit":{"message":"[Actions]
Adjust axios payload for APIs that do not want `data` sent
(#165756)","sha":"d6a2bd77788c318a9f74247a52a1b6da05779b4b"}},"sourceBranch":"main","suggestedTargetBranches":["8.10"],"targetPullRequestStates":[{"branch":"8.10","label":"v8.10.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.11.0","labelRegex":"^v8.11.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/165756","number":165756,"mergeCommit":{"message":"[Actions]
Adjust axios payload for APIs that do not want `data` sent
(#165756)","sha":"d6a2bd77788c318a9f74247a52a1b6da05779b4b"}}]}]
BACKPORT-->

Co-authored-by: Steph Milovic <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:fix Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.10.0 v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants